Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev merge #59

Merged
merged 153 commits into from
Oct 18, 2023
Merged

Dev merge #59

merged 153 commits into from
Oct 18, 2023

Conversation

Jatewo
Copy link
Contributor

@Jatewo Jatewo commented Oct 17, 2023

Description

Added functionality for editing of user accounts, getting courses, and updated the course model.

Changes

Merging of pull requests:

Checklist

  • Code has been tested locally and passes all relevant tests.
  • Documentation has been updated to reflect the changes, if applicable.
  • Code follows the established coding style and guidelines of the project.
  • All new and existing tests related to the changes have passed.
  • Any necessary dependencies or new packages have been properly documented.
  • Pull request title and description are clear and descriptive.
  • Reviewers have been assigned to the pull request.
  • Any potential security implications have been considered and addressed.
  • Performance impact of the changes has been evaluated, if relevant.

Notes for Reviewers

@Jatewo Jatewo self-assigned this Oct 17, 2023
@Jatewo Jatewo marked this pull request as ready for review October 18, 2023 07:26
middlewares/adminOnly.js Outdated Show resolved Hide resolved
middlewares/adminOnly.js Outdated Show resolved Hide resolved
routes/authRoutes.js Outdated Show resolved Hide resolved
MaizeHH
MaizeHH previously approved these changes Oct 18, 2023
routes/authRoutes.js Outdated Show resolved Hide resolved
Copy link
Contributor

@Mafusn Mafusn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the integration team should align the naming of the exported models, because some are just called for example 'component' but others are called 'userModel'.

@MaizeHH MaizeHH dismissed their stale review October 18, 2023 09:01

Accidental

Copy link
Contributor

@MortenMunk MortenMunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Der er nogle filer jeg føler der lige skal dobbeltjekkes, men ikke sikkert det er noget der skal holde pull request tilbage... 😄

__tests__/fixtures/db.js Show resolved Hide resolved
__tests__/routes/courseRoutes.test.js Show resolved Hide resolved
__tests__/routes/courseRoutes.test.js Show resolved Hide resolved
__tests__/routes/courseRoutes.test.js Show resolved Hide resolved
__tests__/routes/courseRoutes.spec.js Outdated Show resolved Hide resolved
__tests__/routes/courseRoutes.test.js Show resolved Hide resolved
models/User.js Show resolved Hide resolved
routes/contentCreatorRoutes.js Outdated Show resolved Hide resolved
routes/courseRoutes.js Outdated Show resolved Hide resolved
@Jatewo
Copy link
Contributor Author

Jatewo commented Oct 18, 2023

I think the integration team should align the naming of the exported models, because some are just called for example 'component' but others are called 'userModel'.

Agreed. Let's bring this up and make changes in the next sprint

@Jatewo
Copy link
Contributor Author

Jatewo commented Oct 18, 2023

I have made some updates and simply replied to some comments. Take a look to see if you agree 👍

@Jatewo Jatewo requested review from MortenMunk and Mafusn October 18, 2023 10:41
Copy link
Contributor

@MortenMunk MortenMunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with comments by @Jatewo, and considerations.
I approve.

Copy link
Contributor

@Mafusn Mafusn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@MortenMunk MortenMunk merged commit 8b1e757 into staging Oct 18, 2023
2 checks passed
@MortenMunk MortenMunk deleted the dev-merge branch October 18, 2023 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants